[SYCL][Doc] Add sycl_ext_intel_maximum_registers spec - #22820
Conversation
Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
| Applications should generally rely on the compiler's default selection unless | ||
| profiling demonstrates a benefit from specifying a different GRF size. | ||
|
|
||
| This extension is similar to link:../experimental/sycl_ext_intel_grf_size.asciidoc[sycl_ext_intel_grf_size] |
There was a problem hiding this comment.
Full disclosure, I copy and pasted the grf_size spec, did a rename from grf_size to maximum_registers, updated the copyright, updated the 'Backend support status' section, and added this line about differences with grf_size.
If there is some other expected difference between the specs please let me know.
There was a problem hiding this comment.
This is a good start, but some other things:
-
If you intend to merge this PR before it is implemented, change the location of the file to the "proposed" directory. In addition, update the "Status" section with the suggested text from the template for a proposed extension.
-
The sentence "A kernel may be defined with at most one of these GRF-setting properties" should be updated in both specs to make it clear that
maximum_registersandgrf_sizeare also mutually exclusive. Maybe something like:The two properties defined below and the properties from sycl_ext_intel_grf_size are all mutually exclusive. A kernel may be decorated with at most one of these properties.
(And make "sycl_ext_intel_grf_size" a link.)
-
Is there error checking now to see if a kernel is decorated with both
grf_sizeandgrf_size_automatic? If so, that error checking should be expanded to all four property when you implement the spec.
There was a problem hiding this comment.
Sorry, I rushed through this too much. All good points, I will implement them.
Is there error checking now to see if a kernel is decorated with both grf_size and grf_size_automatic? If so, that error checking should be expanded to all four property when you implement the spec.
Yep there is and I will do this when I implement the extension, thanks.
Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
gmlueck
left a comment
There was a problem hiding this comment.
Looks good. Just one more comment below.
|
|
||
| == Backend support status | ||
|
|
||
| This extension is not yet implemented. |
There was a problem hiding this comment.
This section doesn't seem very useful for this extension, so I think it can just be deleted.
Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
Add a new specification to select GRF size that will be implemented using a more modern mechanism to communicate the user's specification. It is not yet implemented.